fix(remote): close the heredoc breakout in the bootstrap script - #185

The defect

rubyQuote stops a node name or server URL interpolating Ruby into client.rb — there is already a test for that (TestClientRBDoesNotInterpolateRuby). But the client.rb it produces is then delivered to the target inside a shell heredoc, and nothing guarded that layer.

A newline in either value closes the heredoc early and leaves the remainder to run as shell, through sudo. Actual generated script from a probe:

cat <<'CINC_EOF' | sudo tee '/etc/cinc/client.rb' >/dev/null
chef_server_url 'https://chef.example.com/organizations/acme'
node_name 'web01
CINC_EOF
id > /tmp/pwned          # <- runs as root on the target
cat <<'CINC_EOF' | sudo tee /dev/null
x'
client_key '/etc/cinc/client.pem'

The quoting that exists operates one layer up and cannot see this.

Severity, honestly

The node name usually comes from --node-name, which the operator typed, so in the common case this is self-inflicted. The server URL is not: it comes from the credentials profile, which is exactly the kind of file that gets shared between colleagues, inherited from a previous setup, or checked into a repo.

I would call this hardening rather than a critical hole, but the surrounding code (rubyQuote, shellQuote, the deliberate install -m 0600 before writing the key) shows a clear intent to be injection-safe here, and this is the one gap in that armour.

The fix

Both layers, independently:

1. BootstrapCommand rejects a newline in the node name or server URL. Neither can legitimately contain one, so rejecting beats attempting to escape.
2. writeFileCommand refuses any body containing a line equal to the terminator. Callers validate their own inputs; this makes the heredoc sound regardless of what a future caller passes.

The terminator is now a named constant rather than being spelled out in three places.

Tests

• TestBootstrapCommandRejectsNewlinesInValues — the breakout payload via both the node name and the server URL. Fails on main, where it returns a working exploit script.
• TestWriteFileCommandRejectsDelimiterInContent — the backstop, plus a check that ordinary content is still accepted.

go test ./..., go vet ./..., and gofmt -l . are clean.